Description

A token-like literal (ghp_test123) was added to tests without the required `# pragma: allowlist
secret` annotation. This can trigger secret scanners and violates the repository’s rule for
explicitly annotating allowed test tokens.

Code

webhook_server/tests/test_config_schema.py[R1038-1045]

+        config: dict[str, Any] = {
+            "github-tokens": ["ghp_test123"],
+            "custom-commands": [
+                {"name": "deploy-staging", "description": "Deploy to staging"},
+                {"name": "run-e2e", "description": "Run e2e tests"},
+            ],
+        }
+        validate(instance=config, schema=schema)

Relevance

⭐⭐ Medium

Prior suggestion to use # pragma: allowlist secret for token-like test strings had undetermined
outcome (PR #1081).

PR-#1081

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 181401 disallows hardcoded secret-like strings unless they are explicitly annotated
as allowed test tokens using # pragma: allowlist secret. The test config includes ghp_test123
without the required annotation.

Rule 181401: Disallow hardcoded secrets; explicitly annotate allowed test tokens
webhook_server/tests/test_config_schema.py[1038-1045]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A GitHub-token-shaped test string is committed without the required allowlist annotation comment.

## Issue Context
The compliance rule requires any token-like literals used in tests/examples to be annotated on the same line with `# pragma: allowlist secret`.

## Fix Focus Areas
- webhook_server/tests/test_config_schema.py[1038-1045]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

_prepare_custom_commands_welcome_section directly indexes cmd['name'] and cmd['description'] from
the configured list; if custom-commands is not a list of dicts or any item is missing those keys,
welcome comment generation raises (TypeError/KeyError) and PR-open tasks won’t be scheduled. This
repo’s Config loader does not validate config.yaml against schema.yaml at runtime, so malformed
values can reach this code path.

Code

webhook_server/libs/handlers/pull_request_handler.py[R853-860]

+        custom_commands: list[dict[str, str]] = self.github_webhook.config.get_value("custom-commands", [])
+        if not custom_commands:
+            return ""
+
+        lines: list[str] = ["\n#### Custom Commands"]
+        for cmd in custom_commands:
+            lines.append(f"* `/{cmd['name']}` - {cmd['description']}")
+

Relevance

⭐⭐ Medium

Mixed history: config hardening accepted (PR #978) but some KeyError-hardening suggestions rejected
(PR #1002).

PR-#978
PR-#1002
PR-#980

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new custom-commands rendering indexes dict keys without validation, and the PR-open path builds
the welcome message before creating/scheduling async tasks; any exception prevents those tasks from
being scheduled. Since Config initialization does not perform schema validation, malformed
configuration structures can reach this new code.

webhook_server/libs/handlers/pull_request_handler.py[846-861]
webhook_server/libs/handlers/pull_request_handler.py[235-266]
webhook_server/libs/config.py[13-26]
PR-#978

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`PullRequestHandler._prepare_custom_commands_welcome_section` assumes `custom-commands` is a `list[dict]` with required `name`/`description` keys and indexes them directly. A mis-typed config (e.g., string/object instead of list, or list items missing keys) will raise and prevent `process_pull_request_webhook_data()` from building/scheduling PR-open tasks.

### Issue Context
Runtime config loading does not validate config structure against `config/schema.yaml`, so schema constraints are not sufficient to prevent malformed values from reaching handler code.

### Fix Focus Areas
- webhook_server/libs/handlers/pull_request_handler.py[846-861]
- webhook_server/libs/handlers/pull_request_handler.py[235-266]
- webhook_server/libs/config.py[13-26]

### Implementation notes
- Defensive checks:
 - If `custom_commands` is not a `list`, treat it as empty.
 - For each entry, require `isinstance(cmd, dict)` and `isinstance(name, str)`/`isinstance(description, str)` and both non-empty.
 - Use `cmd.get("name")` / `cmd.get("description")` and skip invalid entries (optionally log debug/warning once).
- Ensure the section header is only emitted if at least one valid command remains.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

CLAUDE.md states that custom-commands: [] at “any layer” disables lower layers, but the root-level
custom-commands schema requires minItems: 1, so an empty list in config.yaml root is
schema-invalid. This can cause confusion when users try to “disable” global custom commands via an
empty root list and hit schema validation failures.

Code

CLAUDE.md[704]

+**Config:** Three resolution layers (first match wins, no list merge): (1) repo-local `.github-webhook-server.yaml`, (2) `repositories.<repo>.custom-commands` in `config.yaml`, (3) root-level `custom-commands` in `config.yaml`. An empty list (`custom-commands: []`) at any layer disables lower layers.

Relevance

⭐⭐⭐ High

Team fixes doc/schema contradictions (accepted CLAUDE.md correction in PR #930); likely align
empty-list behavior/docs/schema.

PR-#930
PR-#1112

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The documentation says an empty custom-commands list works “at any layer”, but the schema sets
minItems: 1 for the root-level custom-commands, which makes custom-commands: [] invalid in
config.yaml root.

CLAUDE.md[704-704]
webhook_server/config/schema.yaml[272-280]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`CLAUDE.md` claims an empty list (`custom-commands: []`) works at any configuration layer, but the schema disallows an empty list at the **root** (global) layer (`minItems: 1`). This is a docs/schema mismatch that may mislead operators.

### Issue Context
- Root-level `custom-commands` in `config.yaml` is constrained by schema (`minItems: 1`).
- Per-repo override explicitly allows an empty list to disable inherited/global commands.

### Fix Focus Areas
- CLAUDE.md[704-704]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

CLAUDE.md says custom-commands is configured only in root config.yaml or repo-local
.github-webhook-server.yaml, but the runtime resolution order also supports
repositories.<repo>.custom-commands in config.yaml. This can mislead operators into putting repo
overrides in the wrong place or assuming server-side per-repo overrides aren’t supported.

Code

CLAUDE.md[704]

+**Config:** Global (`config.yaml`) or per-repo (`.github-webhook-server.yaml`). Per-repo overrides global (no list merge); an empty list (`custom-commands: []`) disables global defaults.

Relevance

⭐⭐⭐ High

Team fixes misleading config docs; accepted correcting per-repo override claims and CLAUDE.md
contradictions (PRs #1109, #930).

PR-#1109
PR-#930
PR-#1112

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The docs line claims only global config.yaml or repo-local .github-webhook-server.yaml, but
Config.get_value() explicitly checks extra_dict (repo-local), then repository_data
(repositories.<repo>), then root_data (global). GithubWebhook._repo_data_from_config() loads
custom-commands through get_value(..., extra_dict=repository_config), so
repositories.<repo>.custom-commands is honored when extra_dict has no value.

CLAUDE.md[702-708]
webhook_server/libs/config.py[132-153]
webhook_server/libs/github_api.py[938-947]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The Custom Commands docs imply there are only two configuration locations (global `config.yaml` and repo-local `.github-webhook-server.yaml`), but the code supports three layers (repo-local file, `config.yaml` `repositories.<repo>`, then root `config.yaml`).

### Issue Context
`Config.get_value()` resolves `extra_dict` first, then `repository_data`, then `root_data`, and `GithubWebhook._repo_data_from_config()` loads `custom-commands` using `extra_dict=repository_config`. This means `repositories.<repo>.custom-commands` in `config.yaml` is a valid per-repo override path and should be documented alongside `.github-webhook-server.yaml`.

### Fix Focus Areas
- CLAUDE.md[704-704]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

CLAUDE.md says per-repo custom-commands “merges with global”, but Config.get_value() returns the
repo-local value directly when present (no list merge), so repo config replaces the global list.
This can lead to incorrect expectations and misconfiguration when users intend to “add” commands on
top of global defaults.

Code

CLAUDE.md[704]

+**Config:** Global (`config.yaml`) or per-repo (`.github-webhook-server.yaml`). Per-repo merges with global; an empty list (`custom-commands: []`) disables global defaults.

Relevance

⭐⭐⭐ High

Team favors docs matching runtime; merge/extend semantics proposal was rejected in PR #1109,
implying override is expected.

PR-#1109
PR-#978

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The documentation claims merge semantics, but Config.get_value() returns extra_dict’s nested
value if it is present (even an empty list), and GithubWebhook._repo_data_from_config() uses that
mechanism to load custom-commands, which results in override/replacement rather than merging.

CLAUDE.md[698-711]
webhook_server/libs/config.py[132-153]
webhook_server/libs/github_api.py[944-948]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`CLAUDE.md` states that per-repo `custom-commands` “merges with global”, but the config resolution path uses `Config.get_value(..., extra_dict=repository_config)` which implements override semantics (repo value wins) and does not merge arrays.

## Issue Context
This is documentation-only, but it affects how users configure multi-repo deployments: they may expect global commands to be automatically included when defining repo-specific commands.

## Fix Focus Areas
- CLAUDE.md[698-711]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

test_call_ai_sidecar_available asserts call_ai_once was called, but not that it was awaited, so
the test can still pass if call_ai() forgets to await the coroutine. This weakens regression
coverage for a common async bug class.

Code

webhook_server/tests/test_ai_cli.py[R62-86]

+            with patch(
+                "webhook_server.libs.ai_cli.call_ai_once",
+                new_callable=AsyncMock,
+                return_value=expected,
+            ) as mock_call:
+                result = await call_ai(
+                    prompt="test",
+                    ai_provider="claude",
+                    ai_model="sonnet",
+                    cwd="/tmp",
+                    timeout_minutes=5,
+                    system_prompt="be helpful",
+                )
+                assert result.success is True
+                assert result.text == "hello"
+                mock_call.assert_called_once_with(
+                    prompt="test",
+                    ai_provider="claude",
+                    ai_model="sonnet",
+                    cwd="/tmp",
+                    ai_call_timeout=5,
+                    system_prompt="be helpful",
+                    tools=None,
+                    custom_tools=None,
+                )

Relevance

⭐⭐⭐ High

Team recently accepted switching AsyncMock assertions to assert_awaited* to catch missing-await
regressions (PR #1117).

PR-#1117

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The test currently uses assert_called_once_with on an AsyncMock, which does not verify the awaited
behavior, while the implementation explicitly awaits call_ai_once.

webhook_server/tests/test_ai_cli.py[53-86]
webhook_server/libs/ai_cli.py[10-38]
PR-#1117

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`call_ai_once` is patched with `AsyncMock`, but the test asserts `assert_called_once_with(...)` instead of `assert_awaited_once_with(...)`. This allows the test to pass even if the production code stops awaiting the coroutine.

### Issue Context
`call_ai()` currently does `return await call_ai_once(...)`, so the test should enforce the await behavior.

### Fix Focus Areas
- webhook_server/tests/test_ai_cli.py[62-86]

### Suggested change
Replace:
- `mock_call.assert_called_once_with(...)`

With:
- `mock_call.assert_awaited_once_with(...)`

(Optional) Also assert `check_sidecar_available` was awaited in both tests if you keep a handle to the patched mock.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

custom-command-item.description is only typed as a string in the schema, so description: ""
passes validation, but _prepare_custom_commands_welcome_section skips entries when description
is empty/falsy. This creates a confusing state where schema-valid configuration silently fails to
render in the PR welcome message.

Code

webhook_server/config/schema.yaml[R78-83]

+      description:
+        type: string
+        description: Human-readable description of what the command does
+    required:
+      - name
+      - description

Relevance

⭐⭐⭐ High

Team often tightens validation to reject empty strings/non-empty requirements (accepted in PR #1117;
similar non-empty enforcement in PR #900).

PR-#1117
PR-#900

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The schema defines description as a plain string with no minLength, while the renderer
explicitly rejects empty/falsy description values (... or not description), so schema-valid
empty descriptions will be dropped at runtime.

webhook_server/config/schema.yaml[71-84]
webhook_server/libs/handlers/pull_request_handler.py[855-869]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The schema allows `custom-commands` entries with an empty string `description`, but the welcome-comment renderer skips commands when `description` is empty/falsy. This leads to schema-valid config that won’t render.

## Issue Context
Runtime rendering explicitly requires `description` to be a non-empty string, so schema should reject empty (and ideally whitespace-only) descriptions to match runtime behavior.

## Fix Focus Areas
- webhook_server/config/schema.yaml[71-84]
- webhook_server/libs/handlers/pull_request_handler.py[855-869]

## Suggested fix
- In `custom-command-item.properties.description`, add `minLength: 1` (and optionally a pattern like `.*\S.*` to reject whitespace-only strings).
- Add/extend a schema test to assert `{"name": "deploy", "description": ""}` fails validation.
- (Optional) Consider `strip()`-based emptiness check in the renderer if you want whitespace-only descriptions treated as missing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

GithubWebhook._repo_data_from_config silently coerces a non-list custom-commands value to [],
which disables the Custom Commands section without any warning. Since runtime config loading does
not validate schema.yaml, this makes production misconfigurations difficult to detect and debug.

Code

webhook_server/libs/github_api.py[R944-949]

+        raw_custom_commands = self.config.get_value(
+            value="custom-commands", return_on_none=[], extra_dict=repository_config
+        )
+        self.custom_commands: list[dict[str, str]] = (
+            raw_custom_commands if isinstance(raw_custom_commands, list) else []
+        )

Relevance

⭐⭐⭐ High

Team usually logs warnings for mis-typed config instead of silent fallback; e.g., trusted-committers
type warning merged.

PR-#1117
PR-#978
PR-#998

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new code assigns custom_commands to an empty list when the configured value is not a list and
does not log; the welcome section is then omitted when the list is empty, masking the
misconfiguration. The codebase already logs this kind of mis-typed config for custom-check-runs,
and the runtime Config class does not perform schema validation (only labels validation), so
schema.yaml tests won’t prevent production misconfigs.

webhook_server/libs/github_api.py[944-949]
webhook_server/libs/handlers/pull_request_handler.py[855-860]
webhook_server/libs/github_api.py[1497-1505]
webhook_server/libs/config.py[13-26]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`custom-commands` is read from config and coerced to `[]` when it is not a list, but this happens silently (no warning). Because the runtime config loader doesn’t validate `schema.yaml`, a mis-typed YAML value (string/object) can reach this code path and the feature will appear to “not work” with no diagnostic signal.

## Issue Context
There is an existing pattern for configuration hardening in this file: `_validate_custom_check_runs(...)` logs a warning and skips invalid config rather than failing silently.

## Fix Focus Areas
- webhook_server/libs/github_api.py[944-949]

### Suggested implementation approach
- Add a small validator for `custom-commands` similar to `_validate_custom_check_runs`:
 - If `raw_custom_commands` is not a list, log a warning including the actual type and fall back to `[]`.
 - (Optional) Filter list elements to dicts only (or leave that to `PullRequestHandler`, but at least warn once if any element is not a dict).
- Use `getattr(self, "log_prefix", "")` for early-init safe log prefix, as done in `_validate_custom_check_runs`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The schema enforces minItems: 1 for per-repo custom-commands, so a repository cannot set
custom-commands: [] to override and disable a global custom-commands default. This conflicts
with the config resolution order (repo scope overrides root) and the handler behavior that treats an
empty list as “no section.”

Code

webhook_server/config/schema.yaml[R740-743]

+        custom-commands:
+          type: array
+          minItems: 1
+          description: |

Relevance

⭐⭐⭐ High

Team previously fixed schema blocking supported per-repo overrides; aligns schema with runtime
precedence (PR #1112).

PR-#1112
PR-#1109
PR-#978

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The schema prohibits empty per-repo lists, but the runtime config resolution and welcome rendering
both support empty-list semantics as an explicit override to produce no section; that means the
schema blocks a supported configuration behavior.

webhook_server/config/schema.yaml[740-766]
webhook_server/libs/config.py[132-153]
webhook_server/libs/handlers/pull_request_handler.py[855-857]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Per-repo `custom-commands` is defined with `minItems: 1`, which prevents setting `custom-commands: []` at repository scope. Because config resolution prefers repo-scoped values over root values, an explicit empty list is the natural way to disable a global default for one repo.

Currently:
- `Config.get_value(...)` returns the repository-scoped value if present, even if it's an empty list.
- The welcome-section renderer treats an empty list as "do not render".
- But the schema rejects `[]`, making this override pattern impossible in schema-valid configs.

## Issue Context
This is specifically about *per-repo overrides* when a global default is configured.

## Fix Focus Areas
- webhook_server/config/schema.yaml[740-743]

## Suggested fix
Adjust the per-repo schema for `custom-commands` to allow an explicit empty list override, for example:
- Remove `minItems: 1` at the repository level (keep it at global/root if you still want to require non-empty when globally configured), or
- Set `minItems: 0` at the repository level, or
- Use a `oneOf` that allows either an empty array or a non-empty array of valid objects.

After the schema change, consider adding a small schema test that validates `custom-commands: []` under a repository config when a global default exists (if your schema test harness supports such combined instances).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

The new global custom-commands schema does not enforce a non-empty list, so an explicitly empty
configuration will still validate even though the compliance requirement calls for validating a
non-empty list of command objects. This can weaken schema enforcement and allow misconfiguration to
pass unnoticed.

Code

webhook_server/config/schema.yaml[R256-275]

+  custom-commands:
+    type: array
+    description: |
+      Custom commands to display in the PR welcome message (global default).
+      These are documentation-only - the server renders them in the welcome
+      message but does NOT process them. External bots or tools handle them.
+    items:
+      type: object
+      properties:
+        name:
+          type: string
+          pattern: "^[a-zA-Z0-9_-]+$"
+          description: Command name (without the leading slash)
+        description:
+          type: string
+          description: Human-readable description of what the command does
+      required:
+        - name
+        - description
+      additionalProperties: false

Relevance

⭐⭐ Medium

No historical evidence enforcing minItems in schema arrays; schema feedback often rejected as
docs/examples (PR #961).

PR-#961

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 181406 requires the schema to validate a non-empty list of objects for
custom-commands. In the new global schema definition, type: array is present but there is no
minItems: 1, so custom-commands: [] would validate.

Add custom-commands configuration option to schema (global + per-repo override)
webhook_server/config/schema.yaml[256-275]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Global `custom-commands` schema allows an empty list, but the co...